Skip to content

fix(security): bind repository scanners to exact PR head - #799

Draft
seonghobae wants to merge 7 commits into
mainfrom
fix/security-scan-exact-head
Draft

fix(security): bind repository scanners to exact PR head#799
seonghobae wants to merge 7 commits into
mainfrom
fix/security-scan-exact-head

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Purpose

Restore exact-head security evidence for organization-required repository scanners. DiskSage PR #137 exact head 87ac0e08cceed3d1a766da13a8f8123912178192 exposed that Security Scan run 31070907732 checked out GitHub's synthetic pull-request merge ref for Trivy instead of the literal contributor head, so the green result could not satisfy exact-head authorization policy.

Implemented boundary

  • dependency-review support, Trivy, and Scorecard check out both the explicit pull-request head repository and immutable head SHA;
  • persisted checkout credentials remain disabled;
  • Trivy and Scorecard SARIF uploads explicitly bind refs/pull/<number>/head and the same exact head SHA;
  • OSV's existing base/head comparison, dependency-review policy, Trivy severity gate, Scorecard soft posture role, permissions, scanner versions, and findings policy remain unchanged;
  • fork pull requests remain read-only and supported through the event's explicit head repository.

TDD evidence

  • RED head 3a1f49a6d5e07b2a3aedd42627579d3e4d3213ce: quality run 31078988647, job 92543220656, failed in Execute dependency-free exact-head contract because the three affected jobs still relied on the synthetic merge checkout.
  • GREEN head aee317b3ecbfaad7cdb3898e603dfe5e69b8f7ae: quality run 31079215588, job 92543948287, passed both checkout and SARIF contracts.
  • Security Scan run 31079215609 passed dependency-review, OSV, Trivy, and Scorecard. Trivy job 92544007320 checked out aee317b3ecbfaad7cdb3898e603dfe5e69b8f7ae and uploaded SARIF as refs/pull/799/head with the same SHA.

Documentation

docs/doctoring/security-scan-exact-head.md records the authorization boundary, fail-closed behavior, rollback contract, initiating DiskSage evidence, and APA 7th references to current authoritative GitHub documentation. CHANGELOG.md records the repair.

Merge gate

Keep Draft until the exact current head passes complete central tests, Python Security, Security Scan, CodeQL, Semgrep, Secret Scan, OSV, Scorecard, SBOM, current-head automated review, independent non-author approval, zero unresolved valid findings, and branch protection without bypass.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 16 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bbc7ed60-4c81-419d-afe7-8f7584fd7787

📥 Commits

Reviewing files that changed from the base of the PR and between f070c50 and aee317b.

📒 Files selected for processing (6)
  • .github/workflows/security-scan-exact-head-quality-ci.yml
  • .github/workflows/security-scan.yml
  • CHANGELOG.md
  • docs/doctoring/security-scan-exact-head.md
  • tests/test_security_scan_exact_head.py
  • tests/test_security_scan_sarif_exact_head.py

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as ready for review August 6, 2026 07:02

Copy link
Copy Markdown
Contributor Author

@opencode-agent Extend this exact-current-head security-scan repair test-first to address #810, and do nothing if PR #799 branch head is no longer aee317b3ecbfaad7cdb3898e603dfe5e69b8f7ae.

A downstream exact-head canary exposed a valid organization-level fail-open defect in the same workflow. ContextualWisdomLab/EgressWeave is public. On EgressWeave PR #66 exact head c038a9509d1a8eae8561cc9081e67e12bd373d42, Security Scan run 31108241013, dependency-review job 92638903658, the exact compare 10d0c51daf2ad278d66f43be479df8cf6b08ba6d...c038a9509d1a8eae8561cc9081e67e12bd373d42 returned HTTP 403; central security-scan.yml warned that dependency review was unavailable, set supported=false, skipped actions/dependency-review-action, and returned a green job/workflow. GitHub currently documents dependency review/action as available for public GitHub.com repositories when dependency graph is enabled, and the public-resource REST compare does not require authentication. Treat this as unavailable security evidence, not a clean dependency review. Do not guess the root cause of the 403.

Use two auditable commits in order:

  1. Immutable RED test-only commit. Modify only tests/test_security_scan_exact_head.py (or add one tightly focused tests/test_security_scan_dependency_review.py if substantially clearer). Add a deterministic static/contract regression that fails on current head because 403/404 are converted to supported=false + exit 0 / success and because the hard action is conditionally skipped. Require the central contract to distinguish an executed clean review from API unavailability and to fail closed for public-repository unavailability. Preserve all existing exact-head checkout and SARIF contracts.

  2. GREEN bounded repair. Modify only .github/workflows/security-scan.yml, the focused test above, docs/doctoring/security-scan-exact-head.md, and CHANGELOG.md. For the dependency-review support probe:

    • keep exact BASE_SHA/HEAD_SHA, exact PR-head checkout, read-only permissions, persisted credentials disabled, and the immutable actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 pin;
    • 200 may continue to the hard dependency-review action;
    • 403, 404, empty/malformed status, curl/transport failure, timeout, and any unexpected status must not yield a successful dependency-review result for a public repository; fail closed with a concise actionable error that identifies repository plus exact base/head and HTTP status but does not print auth material or an untrusted response body;
    • do not use OSV, Trivy, Scorecard, CodeQL, or another scanner as a semantic substitute for dependency review;
    • do not introduce branch-controlled opt-outs or infer not-applicable from 403/404. If private/internal repository exceptions are required, leave that as the separately governed policy decision in security: make central dependency-review unavailability fail closed #810 rather than weakening this public-repository canary repair.

Update doctoring so its claimed fail-closed behavior matches production and include current APA 7 primary references to GitHub Dependency Review, Dependency Graph, and Dependency Review REST API. Add one concise [Unreleased] Security entry; no version/release bump.

Do not change .github/workflows/security-scan-exact-head-quality-ci.yml unless the existing permanent exact-head quality job cannot discover the new focused test without a minimal dependency-free update. Do not modify any other workflow, OpenCode/Noema/Strix credentials or identities, dependencies, repository protections, publication/release/tag/ref behavior, model provider, or caller. Never add a temporary/self-modifying/write-capable repair workflow, contents: write, OIDC, branch push, merge/rebase, auto-merge, -X ours/theirs, or execution of model-modified source under a write credential.

After GREEN, run the focused dependency-free contract and the complete central Python test/100% statement+branch coverage/public-docstring/compile contract available on the branch. Then leave PR #799 unmerged and request fresh exact-GREEN CodeRabbit/OpenCode/Noema review plus every required security workflow; do not claim queued, absent, skipped, predecessor-head, or synthetic-merge evidence as current-head success. Report immutable RED and GREEN SHAs in this PR conversation.

@seonghobae
seonghobae marked this pull request as draft August 6, 2026 21:21

Copy link
Copy Markdown
Contributor Author

@opencode-agent Extend this existing security-scan repair test-first on exact current head aee317b3ecbfaad7cdb3898e603dfe5e69b8f7ae; stop without writing if the head differs.

Fresh downstream evidence from ContextualWisdomLab/EgressWeave#66 exact head c038a9509d1a8eae8561cc9081e67e12bd373d42 exposed a still-valid organization-level fail-open dependency-review path. Security Scan run 31108241013, job 92638903658, queried exact base 10d0c51daf2ad278d66f43be479df8cf6b08ba6d and exact head c038a9509d1a8eae8561cc9081e67e12bd373d42; the dependency-graph compare endpoint returned HTTP 403. The current central step then emitted Dependency review is unavailable for ContextualWisdomLab/EgressWeave; skipping dependency-review hard gate., set supported=false, skipped actions/dependency-review-action, and allowed the dependency-review job and aggregate Security Scan to conclude success. This is tracked as ContextualWisdomLab/EgressWeave#76. PR #66 does not change dependency inputs, so do not claim it introduced a vulnerable dependency; repair the assurance semantics instead.

Use the existing PR #799 rather than creating a competing PR because it already owns .github/workflows/security-scan.yml, its exact-head contracts, doctoring, and CHANGELOG. Preserve the exact-head checkout/SARIF repair already present.

Required RED → GREEN extension:

  1. First commit immutable test-only RED evidence in the existing security-scan contract tests proving that HTTP 403/404 from dependency-review support cannot be represented as a passed dependency-review gate.
  2. Preserve 200 as the only path that may execute the pinned dependency-review action.
  3. For unavailable 403/404, classify the exact PR base..head conservatively and deterministically. If any supported dependency manifest, lock, dependency-submission definition, package-resolution input, or an unknown/truncated classification state is present, fail closed. If and only if the complete exact diff proves no dependency-sensitive path changed, emit an explicit exact-base/exact-head not-applicable outcome; never call that passed.
  4. Any GitHub API enumeration used for classification must paginate fully, bind repository/PR/base/head identities, reject stale heads, and fail closed on malformed responses, page/record limits, truncation, rate/permission failures, or identity drift. Prefer an already-authenticated exact-tree method if it gives stronger completeness evidence. Do not rely on the capped compare-file list as complete evidence.
  5. Keep contents/pull-requests read-only, persist-credentials: false, immutable action pins, fork-read-only behavior, OSV, Trivy, Scorecard, CodeQL/GHAS, Semgrep, Dependabot and existing scanner semantics unchanged except for the corrected dependency-review outcome.
  6. Add focused positive/negative/adversarial tests for dependency-sensitive paths, dependency-neutral paths, stale-head refusal, pagination/completeness, malformed/unavailable responses, and the exact EgressWeave 403 shape. Maintain 100% production statement/branch coverage and beginner-readable public docstrings for any helper added.
  7. Update docs/doctoring/security-scan-exact-head.md with passed vs exact-bound not-applicable vs unavailable/fail-closed, administrator remediation for enabling GitHub dependency graph/Code Security when entitlement permits, and APA 7 primary GitHub references. Prepend one [Unreleased] Security entry; do not bump a version.

Modify only the already-owned PR #799 security-scan workflow/contract/helper/documentation/CHANGELOG surface needed for this repair. Do not add any repository-write, OIDC, branch/ref publication, PR creation/merge, release/package/signing authority, temporary write-capable workflow, self-modifying workflow, model-modified-tree publication, or conflict strategy. Do not merge or mark Ready. After GREEN, require fresh exact-current-head central quality/security/review evidence and preserve independent non-author approval and branch protection as mandatory gates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant